-
Notifications
You must be signed in to change notification settings - Fork 546
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Book Binder #3299
Fix Book Binder #3299
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
-- testing --
do not merge
Could you please cleanup the changelog? |
just remove the changelog changes? |
Sorry for the commit mess |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Would someone want to test these changes again to be sure everything works as intended that way we don't have the mess that was the last enchantment related PR. |
Closing this in favour of #3925 |
Description
Currently there is an issue with the Book Binder.
If you combine 2 books of the same level and the resulting level will go above the set limit (either vanilla or custom) the books will combine into one and keep their current level.
This fix makes it so that if a
Fortune III
gets combined with anotherFortune III
, it will not be combined into anotherFortune III
(granted there is no custom limit)This has no effect for books with multiple enchantments as both books have the same level
Proposed changes
Checklist
Nonnull
andNullable
annotations to my methods to indicate their behaviour for null values